Multiple High Severity Vulnerabilities in Ninja Forms Plugin

This blog post is about vulnerabilities in Ninja Forms plugin vulnerabilities. If you’re a Ninja Forms user, please update the plugin to at least version 3.6.26.

Patchstack Developer and Business users are protected from the vulnerability. You can also sign up for the Patchstack Community plan to be notified about vulnerabilities as soon as they become disclosed.

For plugin developers, we have security audit services and Threat Intelligence Feed API for hosting companies.

About the Ninja Forms plugin

The plugin Ninja Forms versions 3.6.25 and below, free version), which has over 900,000 active installations is known as the more popular forms builder plugin in WordPress. This plugin is developed by Saturday Drive.

This plugin is a free form builder plugin for WordPress that enables us to build just about any type of form we could imagine. Ranging from simple contact forms to event registrations, file uploads, payments, and more.

The security vulnerability

This plugin suffers from multiple vulnerabilities. The first vulnerability is a POST-based reflected XSS. This vulnerability could allow any unauthenticated user to steal sensitive information to, in this case, privilege escalation on the WordPress site by tricking privileged users into visiting the crafted website. The described vulnerability was fixed in version 3.6.26 and assigned CVE-2023-37979.

The second and third vulnerabilities are broken access controls on the form submissions export feature. This vulnerability allows Subscriber and Contributor role users to export all of the Ninja Forms submissions on a WordPress site. The described vulnerability was also fixed in version 3.6.26 and assigned CVE-2023-38393 and CVE-2023-38386.

Reflected XSS

The initial entry point of this vulnerability exists in the route function:

public function route()
{
	register_shutdown_function( array( $this, 'shutdown' ) );

	$method = strtolower( $_SERVER['REQUEST_METHOD'] );

	/*
		* Request Method Override
		* Allows for a POST request to function as another Request Method
		*   by passing a `method_override` value as a request parameter.
		* For example, some servers do not support the DELETE request method.
		*/
	if( 'post' == $method and isset( $_REQUEST[ 'method_override' ] ) ){
		$method = sanitize_text_field( $_REQUEST[ 'method_override' ] );
	}

	if( ! method_exists( $this, $method ) ){
		$this->_errors[] = esc_html__( 'Endpoint does not exist.', 'ninja-forms' );
		$this->_respond();
	}
	/**
	 * This call get the $_REQUEST info for the call(post, get, etc.)
	 * being called.
	 */
	$request_data = $this->get_request_data();

	try {
		$data = $this->$method($request_data);
		$this->_respond( $data );
-------------------------- CUT HERE --------------------------

The above route function could be called from an ajax action named nf_batch_process. Notice on the function that we could override the $method using $_REQUEST[ 'method_override' ] parameter if the original HTTP method performed is a POST request.

The code then will call this->$method() with $request_data as input parameter. The $request_data value is constructed from get_request_data function :

protected function get_request_data()
{
    $request_data = array();

    if (isset($_REQUEST['batch_type']) && $_REQUEST['batch_type']) {
        $request_data['batch_type'] = WPN_Helper::sanitize_text_field($_REQUEST['batch_type']);
    }

    if (isset($_REQUEST['data']) && $_REQUEST['data']) {
        // @TODO: Find a way to safely sanitize this later.
        // sanitize_text_field overcorrects, breaking "actual" data.
        $request_data['data'] = $_REQUEST['data'];
    }

    if (isset($_REQUEST['security']) && $_REQUEST['security']) {
        $request_data['security'] = WPN_Helper::sanitize_text_field($_REQUEST['security']);
    }

    if (isset($_REQUEST['action']) && $_REQUEST['action']) {
        $request_data['action'] = WPN_Helper::sanitize_text_field($_REQUEST['action']);
    }

    return $request_data;
}

The interesting part is which function could we call on the $method variable. Turns out that we could call the _respond function itself which originally act as a wrapper function to output data :

protected function _respond( $data = array() )
{
    if( empty( $data ) ){
        $data = $this->_data;
    }

    if( isset( $this->_data['debug'] ) ) {
        $this->_debug = array_merge( $this->_debug, $this->_data[ 'debug' ] );
    }

    if( isset( $this->_data['errors'] ) && $this->_data[ 'errors' ] ) {
        $this->_errors = array_merge( $this->_errors, $this->_data[ 'errors' ] );
    }

    // allow for accessing and acting on $data before responding
    do_action( 'ninja_forms_before_response', $data );

    $response = array( 'data' => $data, 'errors' => $this->_errors, 'debug' => $this->_debug );

    echo wp_json_encode( $response );

    wp_die(); // this is required to terminate immediately and return a proper response
}

Notice that the function will output $response that could be constructed from our $data variable with echo wp_json_encode().

This will still make HTTP response output as text/html. Looking back at the get_request_data function, we could inject the XSS payload via $_REQUEST['data'] since there is no sanitization or escaping implemented.

Subscriber+ Broken Access Control

The underlying vulnerability exist in the processing function:

public function processing() {

	// Get our passed arguments. These come from the querysting of the processing page.
	if ( isset ( $_REQUEST['args'] ) ) {
		$this->args = WPN_Helper::sanitize_text_field($_REQUEST['args']);
		if ( isset ( $this->args['redirect'] ) ) {
			if( wp_validate_redirect( $this->args['redirect'] ) ){
				$this->redirect = wp_sanitize_redirect( $this->args['redirect'] );
			}
		}
	} else {
		$this->args = array();
	}

	// Get our current step.
	$this->step = isset ( $_REQUEST['step'] )? esc_html( $_REQUEST['step'] ) : 'loading';

	// Get our total steps
	$this->total_steps = isset ( $_REQUEST['total_steps'] )? esc_html( $_REQUEST['total_steps'] ) : 0;

	// If our step is loading, then we need to return how many total steps there are along with the next step, which is 1.
	if ( 'loading' == $this->step ) {
		$return = $this->loading();
		if ( ! isset ( $return['step'] ) ) {
			$saved_step = get_user_option( 'nf_step_processing_' . $this->action . '_step' );
			if ( ! empty ( $saved_step ) ) {
				$this->step = $saved_step;
			} else {
				$this->step = 1;
			}

			$return['step'] = $this->step;
		}
		if ( ! isset ( $return['complete'] ) ) {
			$return['complete'] = false;
		}
	} else { // We aren't on the loading step, so do our processing.
		$return = $this->step();
-------------------------- CUT HERE --------------------------

The processing function is set to be a function handler of nf_download_all_subs ajax action and there is no proper permission check. The function will call $this->step() function which will export all of the submissions to the $this->args['filename'] we specified.

Contributor+ Broken Access Control

The underlying vulnerability exists in the export_listen function:

-------------------------- CUT HERE --------------------------
if (isset ($_REQUEST['download_file']) && !empty($_REQUEST['download_file'])) {

    // Open our download all file
    $filename = esc_html($_REQUEST['download_file']);

    $upload_dir = wp_upload_dir();

    $file_path = trailingslashit($upload_dir['path']) . $filename . '.csv';

    if (file_exists($file_path)) {
        $myfile = file_get_contents($file_path);
    } else {
        $redirect = esc_url_raw(remove_query_arg(array('download_file', 'download_all')));
        wp_redirect($redirect);
        die();
    }

    unlink($file_path);

    $form_name = Ninja_Forms()->form(absint($_REQUEST['form_id']))->get()->get_setting('title');
    $form_name = sanitize_title($form_name);

    $today = date('Y-m-d', current_time('timestamp'));

    $filename = apply_filters('ninja_forms_download_all_filename', $form_name . '-all-subs-' . $today);

    header('Content-type: application/csv');
    header('Content-Disposition: attachment; filename="' . $filename . '.csv"');
    header('Pragma: no-cache');
    header('Expires: 0');

    echo $myfile;

    die();
}
-------------------------- CUT HERE --------------------------

The function is set to be a function handler of load-edit.php hook which could be triggered when a user for example visits the /wp-admin/edit.php endpoint. Since there is no proper permission check, by default, user with the minimum role of Contributor could export form submissions.

The patch

For the reflected XSS issue, the vendor decided to restrict which method the user could access from the function. This patch is enough to prevent the user from directly calling the _respond function to trigger the XSS. The patch can be seen below :

For the Subscriber+ broken access control, adding a permission check should also fix the reported issue. The patch can be seen below :

For the Contributor+ broken access control, adding a permission check should also fix the reported issue. The patch can be seen below :

Conclusion

In some cases, a plugin or theme code needs to call a certain function or class from user supplied string. Always try to check and restrict which function or class the user can directly call. Also, pay extra attention to an export data action and always implement permission or access control checks to the related functions.

Help us make the Internet a safer place

Making the WordPress ecosystem more secure is a team effort, and we believe that plugin developers and security researchers should work together.

• If you’re a plugin developer, join our mVDP program that makes it easier to report, manage, and address vulnerabilities in your software.
• If you’re a security researcher, join Patchstack Alliance to report vulnerabilities & earn rewards.